Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework SCM stageModified status #880

Merged
merged 13 commits into from
Oct 7, 2021

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented Oct 6, 2021

This PR renames the stageModified SCM status to gitModified, splits these resources out into their own group within the SCM tree and names that group "Need Git Commit" in the UI.

For detailed explanation on how/why I arrived at this change take a look here. TL;DR is this means I can drop inline commands from gitModified resources (because they don't make sense) and also it provides a way of showing the user what to do next.

Demo

Screen.Recording.2021-10-07.at.2.31.20.pm.mov

Note: At one stage the above demo exhibits this bug. I click on what should be our commit action but it actually fires the git one 🤸🏻. Luckily this bug is only in insiders for now.

@mattseddon mattseddon added the product PR that affects product label Oct 6, 2021
@mattseddon mattseddon self-assigned this Oct 6, 2021
@@ -1,2 +0,0 @@
export const isStringInEnum = (s: string, E: Record<string, string>) =>
Object.values(E).includes(s)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[F] This is dead now. The only usage is gone.

@mattseddon mattseddon changed the title Split modified and stage modified into separate SCM resource groups Rework SCM stageModified status Oct 7, 2021
@mattseddon mattseddon marked this pull request as ready for review October 7, 2021 04:00
)

this.gitModifiedResourceGroup = this.dispose.track(
scmView.createResourceGroup('gitModified', 'Need Git Commit')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[C] Open to suggestions for this. It's basically a placeholder. There is no action to be performed as far as DVC is concerned.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternative could be Ready for SCM commit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like "Ready for" better than "Need". I'd vote for "Ready for Git commit".

)

this.gitModifiedResourceGroup = this.dispose.track(
scmView.createResourceGroup('gitModified', 'Need Git Commit')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like "Ready for" better than "Need". I'd vote for "Ready for Git commit".

@codeclimate
Copy link

codeclimate bot commented Oct 7, 2021

Code Climate has analyzed commit 3a885ad and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.4% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit 04ba65c into master Oct 7, 2021
@mattseddon mattseddon deleted the remove-checkout-commit-from-staged branch October 7, 2021 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants